Rebase and merge some long-ago written serialization-with-backrefs code. #168
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR rebases long-ago written serialization code to support back‐reference functionality. Key changes include the introduction of an ObjectCache for calculating tree hashes and serialized lengths, refactoring of ReadCacheLookup and serialization routines to handle back-references, and the addition of extensive tests across serialization, object caching, and S-expression parsing.
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| clvm/object_cache.py | Adds ObjectCache class for caching tree hash/length. |
| clvm/read_cache_lookup.py | Implements back-reference lookup functionality. |
| clvm/serialize.py | Refactors serialization logic to support backrefs. |
| tests/read_cache_lookup_test.py | Adds tests covering various lookup and cache behaviors. |
| tests/object_cache_test.py | Introduces tests verifying caching functionality. |
| tests/serialize_test.py | Updates tests to validate back-reference serialization. |
| setup.py | Minor reformatting of package declaration. |
| clvm/SExp.py, clvm/operators.py, clvm/as_python.py, clvm/more_ops.py, clvm/run_program.py, clvm/CLVMObject.py | Minor adjustments and formatting improvements. |
Pull Request Test Coverage Report for Build 14188466227Details
💛 - Coveralls |
There was a problem hiding this comment.
Pull Request Overview
This PR rebases and merges long-ago serialization-with-backrefs code along with several test updates and minor formatting adjustments. Key changes include the introduction of back-reference handling in the serialization/deserialization routines, enhanced read cache lookup operations, and updates to test coverage and error message formatting across modules.
Reviewed Changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| clvm/object_cache.py | Introduces caching logic for CLVM objects without major logic changes. |
| clvm/read_cache_lookup.py | Implements back-reference lookups and updates the pop and push operations. |
| tests/read_cache_lookup_test.py | Adds extensive tests for read cache lookup functionality. |
| tests/object_cache_test.py | Updates tests for the object cache functionality. |
| clvm/serialize.py | Adds support for back-references in serialization along with new helper functions. |
| tests/serialize_test.py | Updates tests to verify serialization with and without back-references. |
| tests/operators_test.py | Minor formatting and consistency updates for operator tests. |
| clvm/as_python.py | Small adjustments to type annotations. |
| tests/cmds_test.py | Adjusts entry point import formatting for tool invocation tests. |
| tests/to_sexp_test.py | Updates formatting and assertions for SExp conversion tests. |
| clvm/CLVMObject.py | Updates error message formatting for tuple size validation. |
| clvm/run_program.py | Minor formatting changes. |
| clvm/more_ops.py | Improves error handling formatting for various eval functions. |
Comments suppressed due to low confidence (2)
clvm/serialize.py:69
- [nitpick] Consider defining enumerated constants or more descriptive names for the operation codes 'P' and 'C' used in the read_op_stack to improve readability and maintainability.
read_op_stack = ["P"]
clvm/CLVMObject.py:46
- [nitpick] Consider using an f-string for the error message for better clarity and consistency, for example: f"tuples must be of size 2, cannot create CLVMObject from: {narrowed_v}".
raise ValueError("tuples must be of size 2, cannot create CLVMObject from: %s" % str(narrowed_v))
richardkiss
force-pushed
the
ser_br
branch
7 times, most recently
from
cf74b55 to
bdb689b
Compare
arvidn
left a comment
arvidn
left a comment
There was a problem hiding this comment.
I consider this more of a reference-implementation than a practical one.
It could probably do with some round-trip tests, including trees where nodes are reused.
Agreed. It's a good baseline for some improvements I'm trying, in particular one that I believe turns an |
No description provided.